fix(server): sanitize unexpected tool errors by uuzzrm · Pull Request #3293 · modelcontextprotocol/python-sdk

uuzzrm · 2026-08-12T00:25:55Z

Fixes #698

Summary

Preserve explicit ToolError messages for expected, model-facing failures.
Log unexpected tool exceptions server-side and return a generic client-facing message.
Keep exception chains and validation details out of tool results.
Update regression coverage, examples, and documentation for the distinction.

Testing

uv run --frozen ruff check on changed Python files
uv run --frozen ruff format --check on changed Python files
uv run --frozen pyright
88 focused tests covering tool execution and documentation examples
The full parallel suite ran 5,576 tests successfully; the remaining xdist timeouts passed when the affected modules were rerun serially.

AI-assisted contribution disclosure

I used AI assistance to inspect the issue and draft the implementation and tests. I reviewed the final diff, ran the checks above, and own the change for review.

cubic-dev-ai

1 issue found across 35 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/server/mcpserver/test_resolve.py">

<violation number="1" location="tests/server/mcpserver/test_resolve.py:1435">
P2: These legacy-mode assertions codify the same client-caused, expected failure (empty accepted elicitation / schema-mismatched accepted answer) being surfaced as a generic "An unexpected error occurred while executing tool ...", while auto mode preserves the specific ToolError message ("received an accepted elicitation with no content" / "does not match the requested schema"). This contradicts the PR's stated goal of preserving explicit ToolError messages for expected, model-facing failures: in legacy mode a bad elicitation response is indistinguishable from a server crash, so the client gets no actionable feedback. The root cause is that the legacy path elicits via ctx.elicit()/elicit_with_validation() which raises a plain ValueError (src/mcp/server/elicitation.py:119,128) caught by the generic handler in tools/base.py:187-189, whereas >= 2026-07-28 raises ToolError in resolve.py:619/624. Consider raising ToolError (or a ValueError subclass the tool runner preserves) from elicit_with_validation so both transports surface the same explicit message.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai · 2026-08-12T00:31:44Z

+            assert "Resolver" in text
        else:
-            assert "Received an accepted elicitation" in text  # the legacy path has no wire key to name
+            assert text == "An unexpected error occurred while executing tool whoami"


P2: These legacy-mode assertions codify the same client-caused, expected failure (empty accepted elicitation / schema-mismatched accepted answer) being surfaced as a generic "An unexpected error occurred while executing tool ...", while auto mode preserves the specific ToolError message ("received an accepted elicitation with no content" / "does not match the requested schema"). This contradicts the PR's stated goal of preserving explicit ToolError messages for expected, model-facing failures: in legacy mode a bad elicitation response is indistinguishable from a server crash, so the client gets no actionable feedback. The root cause is that the legacy path elicits via ctx.elicit()/elicit_with_validation() which raises a plain ValueError (src/mcp/server/elicitation.py:119,128) caught by the generic handler in tools/base.py:187-189, whereas >= 2026-07-28 raises ToolError in resolve.py:619/624. Consider raising ToolError (or a ValueError subclass the tool runner preserves) from elicit_with_validation so both transports surface the same explicit message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/server/mcpserver/test_resolve.py, line 1435: <comment>These legacy-mode assertions codify the same client-caused, expected failure (empty accepted elicitation / schema-mismatched accepted answer) being surfaced as a generic "An unexpected error occurred while executing tool ...", while auto mode preserves the specific ToolError message ("received an accepted elicitation with no content" / "does not match the requested schema"). This contradicts the PR's stated goal of preserving explicit ToolError messages for expected, model-facing failures: in legacy mode a bad elicitation response is indistinguishable from a server crash, so the client gets no actionable feedback. The root cause is that the legacy path elicits via ctx.elicit()/elicit_with_validation() which raises a plain ValueError (src/mcp/server/elicitation.py:119,128) caught by the generic handler in tools/base.py:187-189, whereas >= 2026-07-28 raises ToolError in resolve.py:619/624. Consider raising ToolError (or a ValueError subclass the tool runner preserves) from elicit_with_validation so both transports surface the same explicit message.</comment> <file context> @@ -1425,12 +1428,12 @@ async def whoami(login: Annotated[Login, Resolve(ask)]) -> str: + assert "Resolver" in text else: - assert "Received an accepted elicitation" in text # the legacy path has no wire key to name + assert text == "An unexpected error occurred while executing tool whoami" + assert "errors.pydantic.dev" not in text </file context>

Signed-off-by: Ruiming Zhao <uuzzrm@gmail.com>

uuzzrm · 2026-08-12T00:41:10Z

Thanks for the review. I addressed both findings in commit 5ebaf73.

Legacy elicit_with_validation() now raises ToolError for accepted responses with missing or schema-invalid content, so legacy and auto elicitation paths preserve the same actionable message. Unexpected exceptions remain sanitized by Tool.run().
Corrected the prompt test docstring to describe the actual behavior: the prompt path logs the failure and returns the raw validation message with the stable rendering-error prefix.

Verification:

tests/server/mcpserver/test_resolve.py: 120 passed
tests/interaction/mcpserver/test_prompts.py: passed
tests/server/mcpserver/tools/test_base.py tests/interaction/mcpserver/test_tools.py: 81 passed
ruff check, ruff format --check, and targeted Pyright: passed
git diff --check: passed

The full Pyright run still reports the repository's existing 34 platform-specific typing errors in POSIX/Windows utility and stdio files; none are in the changed files.

AI assistance was used and the final diff and test results were reviewed.

uuzzrm · 2026-08-12T01:40:07Z

The latest CI failure was one stale documentation regression assertion in tests/docs_src/test_elicitation.py:127. It still expected the generic unexpected-tool message, while the legacy elicitation path now intentionally preserves the stable schema-validation ToolError message.

Updated in commit 3898b88 and pushed. Local verification: 179 affected tests passed; Ruff, format check, and git diff --check passed.

AI assistance was used and the final diff and test results were reviewed.

uuzzrm · 2026-08-12T01:54:53Z

I also addressed the standards review findings in 47d4e09:

Documented the intentional broad catch at the Tool.run execution boundary, where arbitrary tool exceptions must be sanitized before crossing MCP.
Added return annotations and behavior/provenance docstrings to the new tests.
Replaced message-regex pytest.raises(..., match=...) assertions with captured exception message assertions.

Verification: 131 affected tests passed; Ruff check, format check, and git diff --check passed. The local Pyright binary is currently blocked by a missing Homebrew libllhttp.9.3.dylib; the previous GitHub Pyright check passed on the PR before this comment-only/test-quality commit.

AI assistance was used and the final diff and test results were reviewed.

uuzzrm · 2026-08-12T02:08:30Z

The latest head also includes the standards-review follow-up in 47d4e09.

The legacy elicitation behavior, stale docs expectation, and test-quality findings are now addressed. The latest CI run 31555163205 is green across the full Python 3.10–3.14 locked/lowest-direct Linux and Windows matrix, including docs, conformance, pre-commit, security analysis, and all-green.

The PR remains open and awaits maintainer review; mergedAt is still null. AI assistance was used and the final diff and test results were reviewed.

github-actions · 2026-08-17T16:26:31Z

Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — CONTRIBUTING.md explains why and how we work. This PR has been closed for now because you aren't currently assigned to #698.

If a maintainer would like this change as a PR from you, they'll assign you to #698 and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.)

There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten.

Maintainers: reopening this PR, removing the missing-issue-link label, or adding bypass-issue-check bypasses the check.

fix(server): sanitize unexpected tool errors

1bea479

uuzzrm mentioned this pull request Aug 12, 2026

Tool.run should not reveal exception value to the client #698

Open

cubic-dev-ai Bot reviewed Aug 12, 2026

View reviewed changes

fix(server): preserve legacy elicitation errors

5ebaf73

Signed-off-by: Ruiming Zhao <uuzzrm@gmail.com>

test(docs): update elicitation error expectation

3898b88

test: clarify sanitized tool error coverage

47d4e09

github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Aug 17, 2026

github-actions Bot closed this Aug 17, 2026

Navigation Menu

Uh oh!

fix(server): sanitize unexpected tool errors - #3293

fix(server): sanitize unexpected tool errors#3293
uuzzrm wants to merge 4 commits into
modelcontextprotocol:mainfrom
uuzzrm:codex/fix-698-tool-error-details

uuzzrm commented Aug 12, 2026

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

cubic-dev-ai Bot Aug 12, 2026

Uh oh!

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

uuzzrm commented Aug 12, 2026

Summary

Testing

AI-assisted contribution disclosure

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

uuzzrm commented Aug 12, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cubic-dev-ai Bot left a comment •

edited

Loading